fix(runner): a quoted shell-error phrase is no longer a runtime error - #997
Merged
Conversation
Closes #992. detect_runtime_error decided whether a failing test also hit a *shell* error by scanning its captured output for phrases like "command not found". That capture includes bashunit's own rendering of the failure, so any message quoting one of those strings was misread and the test reported twice -- once as Failed, once as Error -- for a single cause. It is reachable without touching the framework. A test whose subject is error handling naturally has these strings in its output: asserting that a script prints a useful message, capturing stderr from a command that failed. When such a test fails, the phrase is in the capture and the failure is misclassified. Passing tests were never affected, which is why this stayed invisible until a test started failing -- exactly when clear reporting matters. The discriminator is the prefix bash puts on its own diagnostics: /tmp/rd2.sh: line 2: /etc/passwd: Permission denied Checked across command-not-found, unbound variable, permission denied, no such file, syntax error, division by 0, readonly variable and integer-expression- expected, from inside a sourced test file. All carry ": line N: ". bashunit's own output never does. Two groups, because they are prefixed for different reasons. Parser and expansion diagnostics require the prefix on the same line as the phrase. "killed", "segmentation fault" and "cannot allocate memory" come from job control rather than the parser and can arrive without it, so they keep the old whole-capture match; none of them appears in bashunit's own output, so they carry no false-positive risk. The first attempt applied the prefix rule to everything and broke two existing contracts -- one pinning "process: killed" and one pinning the extracted message's shape. Both were right to fail: the extraction still runs over the whole capture, not the matched line, so the message is byte-identical to before. This also lifts the constraint #991 worked around: assert_true could not say "command not found" without triggering the duplicate. Verified by restoring that wording and confirming no Error appears. The phrasing stays "unknown command" because it is already documented and released, not because it has to be; the comment in assert/core.sh says so now. 1708 sequential / 1667 parallel; baseline + 2, RED first.
bash translates its diagnostics and the classifier's phrase list is English only, so under the Spanish and Japanese CI locales a genuine 'command not found' is never recognised. That is pre-existing -- the list has always been English -- and not what this test is about, so it forces the C locale and keeps testing the classification mechanics. The localization gap is filed separately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤔 Background
Related #992
detect_runtime_errordecided whether a failing test also hit a shell error by scanning its captured output for phrases likecommand not found. That capture includes bashunit's own rendering of the failure, so any message quoting one of those strings was misread and the test reported twice — once asFailed, once asError— for a single cause.It's reachable without touching the framework. A test whose subject is error handling naturally has these strings in its output: asserting a script prints a useful message, capturing stderr from a failed command. Passing tests were never affected, which is why it stayed invisible until a test started failing — exactly when clear reporting matters.
💡 The discriminator
The prefix bash puts on its own diagnostics:
Checked across command-not-found, unbound variable, permission denied, no such file, syntax error, division by 0, readonly variable and integer-expression-expected, from inside a sourced test file. All carry
: line N:. bashunit's own output never does.Two groups, because they're prefixed for different reasons. Parser and expansion diagnostics require the prefix on the same line as the phrase.
killed,segmentation faultandcannot allocate memorycome from job control rather than the parser and can arrive without it, so they keep the whole-capture match — and none appears in bashunit's own output, so they carry no false-positive risk.Applying the prefix rule to everything broke two existing contracts — one pinning
"process: killed", one pinning the extracted message's shape. Both were right to fail. The extraction still runs over the whole capture, not the matched line, so the message is byte-identical to before.🔓 Lifts the #991 workaround
assert_truecouldn't say "command not found" without triggering the duplicate. Verified lifted by restoring that wording and confirming noErrorappears. The phrasing staysunknown commandbecause it's already documented and released, not because it has to be — the comment inassert/core.shnow says so.✅ Verification
2 new acceptance tests (the false positive, and a regression guard that a real shell error is still reported), RED first.
make sa·make lint·bash build.sh bin -v→✅ Build verified ✅· 1708 sequential / 1667 parallel-simple-strict.